Skip to content

fix: support system columns in dataset.take* operations #5722

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
jackye1995 merged 17 commits into from
Jan 28, 2026
Merged

fix: support system columns in dataset.take* operations #5722

jackye1995 merged 17 commits into from
Jan 28, 2026

Conversation

@hamersaw
Copy link
Contributor

@hamersaw hamersaw commented Jan 15, 2026 •
edited by jackye1995
Loading

Previously, "take*" operations did not support _rowid, _rowoffset, _row_created_at_version, and _row_last_updated_at_version. In this PR we add support for all of these columns.

We preserve these system columns through the initial schema projection so that they can be used to populate the correct flags when building the ProjectionPlan and PhysicalProjection structs.

  • _rowid / _rowaddr: persisting these through to ProjectionPlan fields was enough to make them work
  • _rowoffset: required additionally (1) stripping ROW_OFFSET field from ProjectionPlan requested_output_expr and (2) manually injecting column using AddRowOffsetExec (after exposing some methods publicly)
  • _row_created_at_version / _row_last_updated_at_version: required piping through flags to Fragment readers.

Closes #5615.

@hamersaw
adding project_preserve_system_columns function to schema
284ea32 
Signed-off-by: Daniel Rammer <hamersaw@protonmail.com>
@hamersaw
adding flag to existing do_project to furthur simplify
6ecc5b4 
Signed-off-by: Daniel Rammer <hamersaw@protonmail.com>
@hamersaw
typo
ae4b25b 
Signed-off-by: Daniel Rammer <hamersaw@protonmail.com>
@github-actions
Copy link
Contributor

github-actions bot commented Jan 15, 2026

ACTION NEEDED
Lance follows the Conventional Commits specification for release automation.

The PR title and description are used as the merge commit message. Please update your PR title and description to match the specification.

For details on the error please inspect the "PR Title Check" action.

@hamersaw hamersaw changed the title bug: support system columns in take_scan fix: support system columns in take_scan Jan 15, 2026
@github-actions github-actions bot added the bug Something isn't working label Jan 15, 2026
@hamersaw
Merge branch 'main' into bug/support-system-columns-in-take-scan
a4324d0 
Signed-off-by: Daniel Rammer <hamersaw@protonmail.com>
@codecov
Copy link

codecov bot commented Jan 15, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 75.73529% with 33 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
rust/lance-core/src/datatypes/schema.rs 64.91% 18 Missing and 2 partials ⚠️
rust/lance-datafusion/src/projection.rs 40.00% 5 Missing and 1 partial ⚠️
rust/lance/src/dataset/take.rs 89.13% 0 Missing and 5 partials ⚠️
rust/lance/src/io/exec/rowids.rs 85.71% 0 Missing and 2 partials ⚠️

📢 Thoughts on this report? Let us know!

Xuanwo
Xuanwo reviewed Jan 16, 2026
View reviewed changes
Copy link
Collaborator

@Xuanwo Xuanwo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @hamersaw for working on this! Only have a question.

@hamersaw
correct projection for all take_* functions on all system columns
84a995b 
Signed-off-by: Daniel Rammer <hamersaw@protonmail.com>
@hamersaw hamersaw marked this pull request as draft January 19, 2026 15:34
@hamersaw
added AddRow to execution plan - still not working
950659d 
Signed-off-by: Daniel Rammer <hamersaw@protonmail.com>
@hamersaw
manually computing row_offset following row_addr
8dd6547 
Signed-off-by: Daniel Rammer <hamersaw@protonmail.com>
@hamersaw hamersaw changed the title fix: support system columns in take_scan fix: support system columns in dataset::take* operations Jan 20, 2026
@hamersaw
cleaned up
3267635 
Signed-off-by: Daniel Rammer <hamersaw@protonmail.com>
@hamersaw hamersaw changed the title fix: support system columns in dataset::take* operations fix: support system columns in dataset.take* operations Jan 20, 2026
@hamersaw
and more cleanup
e486ada 
Signed-off-by: Daniel Rammer <hamersaw@protonmail.com>
@hamersaw
last cleanup
bcbd005 
Signed-off-by: Daniel Rammer <hamersaw@protonmail.com>
hamersaw
hamersaw commented Jan 20, 2026
View reviewed changes
@hamersaw hamersaw marked this pull request as ready for review January 20, 2026 13:43
jackye1995
jackye1995 reviewed Jan 21, 2026
View reviewed changes
@hamersaw
adding columns before projection to ensure ordering correctness
6c7a205 
Signed-off-by: Daniel Rammer <hamersaw@protonmail.com>
@hamersaw hamersaw requested review from Xuanwo and jackye1995 January 21, 2026 21:30
@jackye1995
Copy link
Contributor

jackye1995 commented Jan 21, 2026

looks like there are quite a few CI failures, could you fix those?

westonpace
westonpace requested changes Jan 23, 2026
View reviewed changes
Copy link
Member

@westonpace westonpace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good but can you add a few tests? Preferably python tests.

@hamersaw
added python tests
99a7403 
Signed-off-by: Daniel Rammer <hamersaw@protonmail.com>
@hamersaw
fix clippy
5ceb80a 
Signed-off-by: Daniel Rammer <hamersaw@protonmail.com>
@hamersaw
fixed existing take_blob apis
a034d30 
Signed-off-by: Daniel Rammer <hamersaw@protonmail.com>
@hamersaw hamersaw requested a review from westonpace January 25, 2026 03:47
westonpace
westonpace approved these changes Jan 27, 2026
View reviewed changes
Copy link
Member

@westonpace westonpace left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome. Thanks for doing this. I'll merge when green.

@westonpace westonpace mentioned this pull request Jan 27, 2026
Open
@westonpace
Copy link
Member

westonpace commented Jan 27, 2026

I've added #5823 which describes some of the stuff we talked about externally.

@westonpace
Copy link
Member

westonpace commented Jan 27, 2026

Looks like some legitimate test failures in the new python tests

@hamersaw
updated python _row_id tests to use the fragment computed offset
4543ee9 
Signed-off-by: Daniel Rammer <hamersaw@protonmail.com>
@hamersaw
updated version tests to use stable row ids
2f4b31e 
Signed-off-by: Daniel Rammer <hamersaw@protonmail.com>
jackye1995
jackye1995 approved these changes Jan 28, 2026
View reviewed changes
Copy link
Contributor

@jackye1995 jackye1995 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me!

@jackye1995 jackye1995 merged commit 37f64ea into lance-format:main Jan 28, 2026
29 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@westonpace westonpace westonpace approved these changes

@jackye1995 jackye1995 jackye1995 approved these changes

@Xuanwo Xuanwo Awaiting requested review from Xuanwo

Assignees

No one assigned

Labels

bug Something isn't working python

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

feat: take_scan can take "_rowid" as meta column

4 participants

@hamersaw @jackye1995 @westonpace @Xuanwo